Conversation
Janther
force-pushed
the
slang-1.1.0
branch
2 times, most recently
from
cfeff2b to
66be80e
Compare
| @@ -16,6 +16,8 @@ Nomic Foundation has put a lot of effort in providing a set of compiler APIs tha | |||
|
|
|||
| Since v2.0.0 this package will ship with the Slang parser and this change must be implemented in existing configurations by replacing `parser: 'solidity-parse'` with `parser: 'slang-solidity'`. | |||
There was a problem hiding this comment.
I wonder if the parser names might confuse users here:
- For the new/default parser, WDYT of just using
slang? since it is the project/NPM package/repository name.. - For the ANTLR parser, WDYT of using
deprecated-solidity-parserinstead ofsolidity-parse? this matches the exact package name, and highlights that it is deprecated.
There was a problem hiding this comment.
My thought here is to keep the same historical name for the old parser in order for nothing to be break (while at least triggering a warning) and allow developers to slowly adopt the new parser.
I'm open to change this if @fvictorio thinks it's a better approach.
There was a problem hiding this comment.
as for the name slang there is a possibility that version 2.1.0 could release with slang-yul alongside slang-solidity. if we feel this is a good feature.
There was a problem hiding this comment.
I see. I now remember the conversation about slang-yul. Let's keep this as-is then. Thanks!
There was a problem hiding this comment.
Yeah, since this is a major version, and I don't think we'll have a 3.0 soon, I'd rather have that breaking change now than keep names that are not great. Especially because setting the parser is more likely to happen now. I'll work on this on a separate PR.
|
|
||
|
|
||
| function() { | ||
| function hello() { |
There was a problem hiding this comment.
Q: Is this due to the Slang upgrade?
There was a problem hiding this comment.
the solidity version needed to be 0.8.29 for the storage layout. but this version doesn't support nameless functions.
| test('should use the latest successful version if the source has no pragmas', function () { | ||
| createParser(`pragma solidity 0.8.28;`, options); | ||
| // This is to create in memory the latest parser and review the behaviour | ||
| createParser(`pragma solidity ${latestSupportedVersion};`, options); |
There was a problem hiding this comment.
Not blocking of course, but just FYI: the newly shipped Languagefacts.inferSupportedVersions() can now replace most of the custom implementation in createParser()...
There was a problem hiding this comment.
I'm aware of this but was very busy moving out of my old house, travelling to Europe, and for the next few weeks I'll be AFK. So it didn't feel healthy to add more stress with an extra feature.
When I return to society I'll tackle this one.
There was a problem hiding this comment.
Of course. Thanks for all the great work so far!
OmarTawfik
left a comment
OmarTawfik
left a comment
There was a problem hiding this comment.
Left a couple of suggestions. Otherwise, LGTM!
cc @fvictorio
3 participants
Adding support for storage layout.